Fix release governance and offline readiness blockers#655
Conversation
📝 WalkthroughWalkthroughThis PR adds staging cross-tenant regression testing, offline release verification, registry governance reconciliation, clinical review queue generation, RAG route deadlines and evaluation diagnostics, plus UI persistence, navigation, loading, and browser-test updates. ChangesStaging isolation and offline release
Governance, evaluation, and RAG controls
UI loading and interaction reliability
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ca2f9f372
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@copilot resolve the merge conflicts on this branch. |
…-remediation # Conflicts: # docs/branch-review-ledger.md
|
@copilot resolve the merge conflicts on this branch. |
Resolved merge conflict in docs/branch-review-ledger.md by keeping all entries from both branches in chronological order.
Head branch was pushed to by a user without write access
…n' into codex/release-blocker-remediation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/clinical-dashboard/global-search-shell.tsx (1)
2-3: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep
ClinicalDashboardcode-splitStatic importing
ClinicalDashboardpulls the 4k-line dashboard module into everyGlobalSearchShellroute, even when the standalone shell branch renders. Restore the lazy import if you want to preserve the old bundle split.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/clinical-dashboard/global-search-shell.tsx` around lines 2 - 3, Update GlobalSearchShell’s ClinicalDashboard dependency to use the existing lazy/dynamic import pattern instead of a static import, preserving code-splitting so standalone shell routes do not load the dashboard module. Keep the ClinicalDashboard rendering behavior unchanged when that branch is selected.Source: Path instructions
tests/ui-smoke.spec.ts (1)
56-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest helper relies on undocumented React DOM internals.
waitForReactEventHandlerdetects handler attachment via__reactProps$*, an internal fiber-props property name that React does not guarantee across versions. This is a P2/test-quality concern (not a functional bug today) surfaced once here rather than at each of its five call sites (Lines 2860, 2905-2906, 2936, 2967, 3008), per the file's JS/TS review policy of limiting inline findings and consolidating P2 context into one comment.Consider a stable alternative that doesn't depend on React internals, e.g. having the component set a
data-scroll-readyattribute (or similar) once its scroll handler is mounted, and polling that instead.As per coding guidelines,
**/*.{js,jsx,ts,tsx,mjs,cjs}review policy states: "Automatic reviews may emit at most three inline findings, and inline comments are limited to P0/P1 issues; P2 context belongs in one summary," which is why this single note covers allwaitForReactEventHandlerusages in this file rather than repeating per call site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ui-smoke.spec.ts` around lines 56 - 69, Replace the undocumented React-props inspection in waitForReactEventHandler with a stable readiness signal exposed by the relevant component, such as a data-scroll-ready attribute set when its scroll handler is mounted. Update the helper to poll that attribute through the Locator API, preserving its timeout and success behavior, and ensure all existing call sites continue using the helper unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test-cross-tenant-staging.ts`:
- Around line 471-486: Ensure the successful reindex path in the staging harness
does not proceed directly to cleanup after the 201 response: either poll the
queued reindex job until it reaches a terminal state, or skip executing the real
reindex while retaining the status-code assertion if only response validation is
needed. Update the flow around the second requestJson call and the “reindex”
checkpoint.
In `@src/components/ClinicalDashboard.tsx`:
- Line 756: Move the activeAnswerThreadOwnerIdRef.current assignment out of the
ClinicalDashboard render path and into the existing effect that responds to
answerThreadOwnerId changes, alongside previousAnswerThreadOwnerIdRef updates.
Preserve the current assignment timing and owner-gating behavior while ensuring
both refs are updated only after the render commits.
In `@src/components/DocumentViewer.tsx`:
- Around line 2581-2583: Update the onPageChange handler in DocumentViewer to
navigate through the existing Next.js router/state mechanism instead of calling
window.history.pushState. Ensure page changes update selectedPage and all
dependent UI, including the subtitle and useHideOnScroll, while preserving the
documentPageHref destination.
---
Nitpick comments:
In `@src/components/clinical-dashboard/global-search-shell.tsx`:
- Around line 2-3: Update GlobalSearchShell’s ClinicalDashboard dependency to
use the existing lazy/dynamic import pattern instead of a static import,
preserving code-splitting so standalone shell routes do not load the dashboard
module. Keep the ClinicalDashboard rendering behavior unchanged when that branch
is selected.
In `@tests/ui-smoke.spec.ts`:
- Around line 56-69: Replace the undocumented React-props inspection in
waitForReactEventHandler with a stable readiness signal exposed by the relevant
component, such as a data-scroll-ready attribute set when its scroll handler is
mounted. Update the helper to poll that attribute through the Locator API,
preserving its timeout and success behavior, and ensure all existing call sites
continue using the helper unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 935d11c9-889a-4271-a1a6-a4f0b6a6049e
📒 Files selected for processing (48)
.env.example.github/workflows/staging-tenancy.ymldocs/branch-review-ledger.mddocs/process-hardening.mddocs/staging-setup.mddocs/staging-tenancy-release-evidence.mdpackage.jsonscripts/build-clinical-review-queue.tsscripts/check-document-label-coverage.tsscripts/classify-documents.tsscripts/eval-quality.tsscripts/production-readiness.tsscripts/reconcile-registry-governance.tsscripts/run-eval-safe.mjsscripts/test-cross-tenant-staging.tsscripts/verify-release-offline.mjssrc/app/page.tsxsrc/components/ClinicalDashboard.tsxsrc/components/DocumentViewer.tsxsrc/components/clinical-dashboard/answer-result-surface.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/use-hide-on-scroll.tssrc/components/document-viewer/pdf-canvas-viewer.tsxsrc/lib/answer-thread-storage.tssrc/lib/clinical-review-queue.tssrc/lib/health-response.tssrc/lib/rag-route-budget.tssrc/lib/rag.tssrc/lib/registry-corpus.tssrc/lib/types.tstests/answer-progress-ui-smoke.spec.tstests/answer-thread-storage.test.tstests/classify-documents.test.tstests/clinical-review-queue.test.tstests/cross-tenant-staging-config.test.tstests/document-label-coverage.test.tstests/eval-quality.test.tstests/health-route.test.tstests/offline-release-profile.test.tstests/playwright-scroll.tstests/production-readiness-offline.test.tstests/rag-answer-fallback.test.tstests/rag-route-budget.test.tstests/registry-corpus.test.tstests/registry-governance-reconciliation.test.tstests/retrieval-owner-filter-guard.test.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98aaad43d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b152ed1f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/DocumentViewer.tsx (1)
2613-2622: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
onPageChangebreaksPdfCanvasViewer's memoization.The inline arrow function passed as
onPageChangegets a new identity on everyDocumentViewerrender, undermining the exact optimization documented a few lines above (handleSignedUrlExpired/handlePdfLoadSuccessareuseCallback-wrapped specifically "so the memoised PdfCanvasViewer isn't re-rendered — and its page re-rastered — every time an unrelated parent state ... changes"). Now a source-search keystroke, composer focus toggle, or online/offline flip will re-renderPdfCanvasVieweragain.🐛 Proposed fix
+ const handlePageChange = useCallback( + (page: number) => { + window.history.pushState(null, "", documentPageHref(documentId, page)); + }, + [documentId], + ); + ... <PdfCanvasViewer key={`${documentId}-${useNativePdfViewer ? "native" : "canvas"}`} url={signedUrl} title={documentDisplayTitle(document)} initialPage={initialPage} onUrlExpired={handleSignedUrlExpired} onLoadSuccess={handlePdfLoadSuccess} - onPageChange={(page) => { - window.history.pushState(null, "", documentPageHref(documentId, page)); - }} + onPageChange={handlePageChange} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DocumentViewer.tsx` around lines 2613 - 2622, Stabilize the onPageChange prop passed to PdfCanvasViewer by defining the history-update handler with useCallback in DocumentViewer, preserving the existing documentPageHref behavior and dependencies. Replace the inline arrow function with this memoized handler so unrelated DocumentViewer renders do not invalidate PdfCanvasViewer memoization.
🧹 Nitpick comments (1)
src/components/DocumentViewer.tsx (1)
1629-1632: 📐 Maintainability & Code Quality | 🔵 TrivialMinor: the conditional spread for
scrollContaineris unnecessary.
useHideOnScroll'sscrollContaineralready defaults tonull, soscrollContainer: shellScrollContainer(which isHTMLElement | null) is equivalent to the conditional-spread form. Simplifying removes an indirection with no behavior change.♻️ Optional simplification
- const scrollHidden = useHideOnScroll({ - ...(shellScrollContainer ? { scrollContainer: shellScrollContainer } : {}), - resetKey: `${documentId}:${initialPage}:${chunkId ?? ""}`, - }); + const scrollHidden = useHideOnScroll({ + scrollContainer: shellScrollContainer, + resetKey: `${documentId}:${initialPage}:${chunkId ?? ""}`, + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DocumentViewer.tsx` around lines 1629 - 1632, Update the useHideOnScroll call to pass scrollContainer: shellScrollContainer directly, removing the conditional spread while preserving the existing resetKey and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-document-label-coverage.ts`:
- Around line 212-219: Centralize the registry-document predicate currently
implemented by isRegistryDocument in scripts/check-document-label-coverage.ts
and its counterpart in scripts/classify-documents.ts. Add one shared helper
accepting source_kind, source_path, file_name, and file_type, preserve the
existing registry-projection checks, and update both call
sites—scripts/check-document-label-coverage.ts:212-219 and
scripts/classify-documents.ts:161-168—to use it.
In `@scripts/eval-quality.ts`:
- Around line 287-293: Update configureEvalProviderEnvironment to make the
parsed providerMode authoritative: when it is "openai", explicitly set
process.env.RAG_PROVIDER_MODE to "openai" even if the environment currently
contains "offline", while preserving the existing offline credential cleanup.
Add a focused test covering openai selection with an ambient offline value, and
run the narrowest relevant test validation.
In `@scripts/test-cross-tenant-staging.ts`:
- Around line 587-605: Update the createClient calls for clientA, clientB, and
admin to use a bounded timeout-aware global.fetch through their client options.
Preserve the existing auth settings and apply the same transport configuration
to all three Supabase clients so sign-in, fixture writes, and cleanup cannot
hang indefinitely.
In `@src/components/ClinicalDashboard.tsx`:
- Around line 565-566: Update the answer-thread owner-switch flow in
ClinicalDashboard so persistence is gated by a committed owner that advances
only after the reset/restore microtask completes, rather than by
activeAnswerThreadOwnerIdRef.current during render. Explicitly clear the
previous owner during the switch, and ensure the persistence effect cannot write
the previous thread under the new owner until bootstrapping finishes.
In `@src/components/DocumentViewer.tsx`:
- Around line 2620-2622: Update the onPageChange handler in DocumentViewer to
avoid desynchronizing page state from browser history: use replaceState if
per-page Back/Forward entries are not intended, or implement popstate handling
that reads the URL page and restores initialPage/viewer state when navigating
history. Preserve the existing documentPageHref URL generation.
In `@src/lib/rag.ts`:
- Around line 3592-3593: Guard the unsupported-answer cache call near
setCachedAnswer with !routeDeadline.deadlineExceeded, and apply the same
condition to the generated-answer cache call in src/lib/rag.ts at lines
4494-4495. Add a targeted test covering an answer that exceeds its deadline
after generation and verifying it is not cached.
- Around line 3451-3455: Move deadline creation to the request-level flow before
the retrieval wait, specifically before searchChunksWithTelemetry and routing
execute, so the caller signal can abort stalled retrieval. Update the
routeDeadline setup around createAnswerRouteDeadline to derive the selected
route’s remaining budget from that earlier request deadline while preserving the
existing route.mode and startedAt inputs.
- Around line 3475-3476: Update the unsupported-route branch around route.mode
=== "unsupported" to catch deadline expiry from
routeDeadline.race(relatedDocumentsPromise), return the already-available
unsupported answer, and still dispose the related-document lookup resources. Add
the smallest targeted test covering a related-document lookup that outlives the
unsupported-route budget and verifies successful recovery plus disposal.
In `@src/lib/registry-corpus.ts`:
- Around line 243-317: The registry reconciliation flow must not overwrite
reviewer-managed generated-label metadata or confidence. Update
reconcileRegistryGeneratedLabels and its related reconciliation path to preserve
existing document_intent metadata and reviewer changes while updating only
generator-owned fields, using an existing-label metadata merge or a narrowly
scoped upsert.
---
Outside diff comments:
In `@src/components/DocumentViewer.tsx`:
- Around line 2613-2622: Stabilize the onPageChange prop passed to
PdfCanvasViewer by defining the history-update handler with useCallback in
DocumentViewer, preserving the existing documentPageHref behavior and
dependencies. Replace the inline arrow function with this memoized handler so
unrelated DocumentViewer renders do not invalidate PdfCanvasViewer memoization.
---
Nitpick comments:
In `@src/components/DocumentViewer.tsx`:
- Around line 1629-1632: Update the useHideOnScroll call to pass
scrollContainer: shellScrollContainer directly, removing the conditional spread
while preserving the existing resetKey and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 935d11c9-889a-4271-a1a6-a4f0b6a6049e
📒 Files selected for processing (50)
.env.example.github/workflows/staging-tenancy.ymldocs/branch-review-ledger.mddocs/process-hardening.mddocs/staging-setup.mddocs/staging-tenancy-release-evidence.mdpackage.jsonscripts/build-clinical-review-queue.tsscripts/check-document-label-coverage.tsscripts/classify-documents.tsscripts/eval-quality.tsscripts/production-readiness.tsscripts/reconcile-registry-governance.tsscripts/run-eval-safe.mjsscripts/test-cross-tenant-staging.tsscripts/verify-release-offline.mjssrc/app/page.tsxsrc/components/ClinicalDashboard.tsxsrc/components/DocumentViewer.tsxsrc/components/clinical-dashboard/answer-result-surface.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/use-hide-on-scroll.tssrc/components/document-viewer/pdf-canvas-viewer.tsxsrc/instrumentation.tssrc/lib/answer-thread-storage.tssrc/lib/clinical-review-queue.tssrc/lib/health-response.tssrc/lib/rag-route-budget.tssrc/lib/rag.tssrc/lib/registry-corpus.tssrc/lib/types.tstests/answer-progress-ui-smoke.spec.tstests/answer-thread-storage.test.tstests/classify-documents.test.tstests/clinical-review-queue.test.tstests/cross-tenant-staging-config.test.tstests/document-label-coverage.test.tstests/eval-quality.test.tstests/health-route.test.tstests/instrumentation.test.tstests/offline-release-profile.test.tstests/playwright-scroll.tstests/production-readiness-offline.test.tstests/rag-answer-fallback.test.tstests/rag-route-budget.test.tstests/registry-corpus.test.tstests/registry-governance-reconciliation.test.tstests/retrieval-owner-filter-guard.test.tstests/ui-smoke.spec.tstests/ui-tools.spec.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 978d4f462f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ece1b891e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
RAG_PROVIDER_MODE=offline.Verification
npm run verify:pr-local— aggregate attempt reached format/lint/typecheck, then the Vitest worker exited without an assertion summary. Its constituent gates passed independently as listed below.npm run verify:cheap— 254 files passed, 1 skipped; 2,325 tests passed, 1 skipped.npm run verify:ui— Chromium and Chromium mockups, 163/163 passed.npm run eval:rag:offline— 36 fixtures; 21 files / 280 tests passed.npm run check:github-actions,npm run check:ci-scope, format, typecheck, and diff checks passed.npm run eval:quality:releaseandnpm run verify:release— intentionally deferred; they require OpenAI credits and remain unchanged.npm run check:production-readinessagainst live production — local offline behavior passed the missing-OpenAI condition and failed only on intentionally absent Supabase production variables. Live identity/readiness is a pre-merge gate.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy); the reconciliation has an expected-project-ref guard.Release notes and remaining gates
Summary by CodeRabbit